-
Notifications
You must be signed in to change notification settings - Fork 25
Tracking TotalInducedVoltage within FullRingAndRF Tracker #217
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Tracking TotalInducedVoltage within FullRingAndRF Tracker #217
Conversation
… in the tracking, but it does not update the induced voltage class itself, i.e. if you don't track the induced voltage, it will remain constant. This adds the updating of the induced voltage into the FullRingAndRFTracker, such that if the TotalInducedVoltage class is passed to the tracker, all updates will be done within the FullRingAndRFTracker. This means you do not need to add TotalInducedVoltage classes to the map and track them seperately in the simulation.
|
I disagree. Calculating the induced voltage can be computationally heavy, and something like updating it every other turn can be a viable option. This would also remove the ability to modify the induced voltage after it's calculated but before it is applied to the beam, e.g. to test new types of feedback calculations. My suggestion would be to either leave it as is, or have a flag to indicate if you want the sum to be calculated as part of the |
|
I see your point. However, that functionality would still be avaliable by not including the TotalInducedVoltage object in the RF Tracker and just calling the track() method of TotalInducedVoltage whenever desired. My main problem right now is readability: There is an option to include the induced voltage in FullRingAndRF, however this, to me, has a confusing effect: The induced voltage is simply included in the kick of the beam particles. This suggests that to update the induced voltage, one should call track() on the voltage seperately. However, the track() method of the voltage updates it AND does the kick with the induced voltage. I.e. including the TotalInducedVoltage object in FullRingAndRF will make the simulation actively incorrect if you also track the TotalInducedVoltage seperately, as this will apply a kick with the induced voltage twice. At least this is the case with my current understanding of the code. Now this change would provide a use case for both approaches: If one includes the TotalInducedVoltage in the FullRingAndRF, it will simply be taken care of automatically. If one wishes to fine tune it, they can leave it out and do the tracking on the voltage itself. To me, this makes more sense. |
|
Interesting point. I will need convincing, but I think you can make a good case for it. Run it past @kiliakis and maybe it would be worth having a brief discussion on it at the next devs meeting. |
|
I agree that the existing implementation is a bit confusing, and it is very common for new users to accidentally apply the induced voltage kick twice (by tracking the TotalInducedVoltage object in the main loop and also passing it to the constructor of the FullRingAndRF). For me the design problem is that the track method of TotalInducedVoltage is not only updating TotalInducedVoltage attributes (i.e. the self.induced_voltage array), but also the beam coordinates, that are updated by the tracker too. I think it would be best to only allow the tracker (FullRingAndRF) to update the beam coordinates. In that case, the track() method of TotalInducedVoltage should only calculate the self.induced_voltage array. Then the energy kick coming from the induced voltage would be applied by the FullRingAndRF tracker (by passing the TotalInducedVoltage attribute in the constructor). What do you think? We can discuss the 3 options (1. leave as is, 2. merge Oleks PR, 3. TotalInducedVoltage track only updates the induced voltage) in the next blond meeting. |
|
My favourite is option 3. I think we want to preserve the ability to "interfere", but that should solve the problem about accidentally kicking twice when using the |
|
I also think option 3 is best. It removes the easy mistake of updating the
beam via tracking the induced voltage, while keeping with the idea that
every major object should be tracked.
Besides, I am not going to be able to make it to the meeting, but please do
agree on something without me.
scpalbright ***@***.***> schrieb am Mo., 17. Apr. 2023, 13:38:
… My favourite is option 3. I think we want to preserve the ability to
"interfere", but that should solve the problem about accidentally kicking
twice when using the track function.
—
Reply to this email directly, view it on GitHub
<#217 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/A3HQKKMRKFXAFVYPPPI454TXBUTTFANCNFSM6AAAAAAW5EEDFI>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
Another point I noticed when working with multiple RFSections: I think the kick modifying beam coordinates was originally put into the TotalInducedVoltage directly, because it affects the entire beam throughout the beampipe, while the kicks of the RF sections are done one after the other, but each RF section, I think, takes into account the entirety of the induced voltage, without adjusting for its own position in the pipe. At least that is how the code looks to me. So this might be worth discussing in more detail. Perhaps putting the kick given by the induced voltage into the FullRingandRF tracker might also be a good idea to get around this problem? I am not really sure here |
The FullRingandRFTracker has an option to include the induced voltage in the tracking, but it does not update the induced voltage class itself, i.e. if you don't track the induced voltage, it will remain constant. To me this seems to not have any use case, so the following change might be useful:
This adds the updating of the induced voltage into the FullRingAndRFTracker, such that if the TotalInducedVoltage class is passed to the tracker, all updates will be done within the FullRingAndRFTracker. This means you do not need to add TotalInducedVoltage classes to the map and track them seperately in the simulation, improving readability and reducing overhead.